-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[mlir][spirv] Add mlir-spirv-tests CI to run for mlir-spv target tests #152124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-github-workflow Author: Davide Grohmann (davidegrohmann) ChangesFull diff: https://github.com/llvm/llvm-project/pull/152124.diff 1 Files Affected:
diff --git a/.github/workflows/spirv-tests.yml b/.github/workflows/spirv-tests.yml
index f15ca1cb64ba5..a2bb64d400bbf 100644
--- a/.github/workflows/spirv-tests.yml
+++ b/.github/workflows/spirv-tests.yml
@@ -23,7 +23,7 @@ jobs:
name: Test SPIR-V
uses: ./.github/workflows/llvm-project-tests.yml
with:
- build_target: check-llvm-codegen-spirv
+ build_target: check-llvm-codegen-spirv check-mlir
projects:
- extra_cmake_args: '-DLLVM_TARGETS_TO_BUILD="SPIRV" -DLLVM_INCLUDE_SPIRV_TOOLS_TESTS=ON'
+ extra_cmake_args: '-DLLVM_TARGETS_TO_BUILD="X86;SPIRV" -DLLVM_INCLUDE_SPIRV_TOOLS_TESTS=ON'
os_list: '["ubuntu-24.04"]'
|
28e45e5
to
2acd645
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if it would be better to bifurcate spirv-tests.yml
and have a separate version for mlir-spirv. It's very rare that we have PRs that affect both implementations, so having them separate could save us some CI time.
2acd645
to
22ca55f
Compare
Yes it is likely better. This is now done. |
This should execute also the MLIR SPIRV Target tests which require the SPIRV-Tools validation Signed-off-by: Davide Grohmann <[email protected]> Change-Id: Ied323152e36b95d6f21ed7d4ce4f5f813f280f17
22ca55f
to
0301179
Compare
Signed-off-by: Davide Grohmann <[email protected]> Change-Id: I52a5fcf79aba2c272a2ac6a4dc2c393a02b398d9
- add SPIRV dialect definition and implementation - add serialization/deserialization implementation Signed-off-by: Davide Grohmann <[email protected]> Change-Id: Ib63045fec041ba7a2d52b527589c30157ed56258
Signed-off-by: Davide Grohmann <[email protected]> Change-Id: I9919a359c8fbefbc5dd030f9a19c7a83fab12682
I'm going to merge once we confirm that the new check works as intended: https://github.com/llvm/llvm-project/actions/runs/16807047739/job/47602153053?pr=152124 |
MLIR SPIR-V tests seem to take ~1h (https://github.com/llvm/llvm-project/actions/workflows/mlir-spirv-tests.yml) compared to regular checks that finish in ~5-10min. I wonder if there is any reason for that and whether we can bring the time down? |
I can have a look. Can you point me to the workflows scripts for the regular checks so I can compare those with the mlir spir-v tests script? |
Probably no build cache? When I built this configuration locally, a lot of time was spent during spirv-tools checkout. I wonder if we can optimize how this gets cloned, e.g., |
I believe it's "CI Checks" (https://github.com/llvm/llvm-project/actions/workflows/premerge.yaml), script: https://github.com/llvm/llvm-project/blob/main/.github/workflows/premerge.yaml |
I have also noticed that auto-merge doesn't wait for this job: #153440 We also should address that if deemed necessary. Not sure if this PR is the best place to track those issues. |
This should execute also the MLIR SPIRV Target tests which require the SPIRV-Tools validator